Skip to content

feat: directoryExists#137

Closed
ChiragAgg5k wants to merge 1 commit intomainfrom
feat-dir-exists
Closed

feat: directoryExists#137
ChiragAgg5k wants to merge 1 commit intomainfrom
feat-dir-exists

Conversation

@ChiragAgg5k
Copy link
Copy Markdown
Member

@ChiragAgg5k ChiragAgg5k commented Oct 1, 2025

Summary by CodeRabbit

  • New Features

    • Added a directory existence check across storage backends (Local and S3), enabling apps to verify whether a directory/path prefix is present before performing operations.
  • Tests

    • Added comprehensive tests for directory existence on Local and S3, including existing, non-existing, and nested directories, with setup and cleanup to ensure reliability.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Oct 1, 2025

Walkthrough

Adds a new directoryExists(string $path): bool API to the storage abstraction. Implements it in Local and S3 devices. S3 checks by listing objects with a normalized directory prefix. Updates unit tests for Local and S3, and adds an abstract getAdapterType() method to S3Base tests.

Changes

Cohort / File(s) Summary of changes
Storage API contract
src/Storage/Device.php
Added abstract method directoryExists(string $path): bool to the base Device class.
Local device implementation
src/Storage/Device/Local.php
Implemented directoryExists(string $path): bool for local filesystem.
S3 device implementation
src/Storage/Device/S3.php
Implemented directoryExists(string $path): bool using prefix-based object listing (normalized to trailing slash), returning true if any object found; false on exceptions.
Local tests
tests/Storage/Device/LocalTest.php
Added testDirectoryExists() covering existing, non-existing, create-and-check, and cleanup scenarios.
S3 tests and base
tests/Storage/S3Base.php
Added abstract getAdapterType(): string. Added testDirectoryExists() validating existing, non-existing, nested directories, with setup/cleanup.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Client
  participant Device as Device (interface)
  participant Local as Local
  participant S3 as S3
  Note over Device: New API: directoryExists(path)

  Client->>Device: directoryExists(path)
  alt Local adapter
    Device->>Local: directoryExists(path)
    Local-->>Device: boolean (filesystem dir check)
  else S3 adapter
    Device->>S3: directoryExists(path)
    Note right of S3: Normalize path to have trailing "/"<br/>ListObjectsV2 prefix=path limit=1
    S3-->>Device: boolean (any object with prefix)
    Note over S3: On exception → return false
  end
  Device-->>Client: boolean
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I burrow through bytes, ears tall and bright,
A new path check—does the dir exist tonight?
Local paws feel folders near,
S3 sniffs prefixes clear.
Hop, list, confirm—no fuss, no fright,
Then thump the tests: all green delight! 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly describes the primary addition of the new directoryExists feature across storage devices, clearly summarizing the main change in a concise and conventional commit format.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat-dir-exists

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 71102ea and a53c0ea.

📒 Files selected for processing (5)
  • src/Storage/Device.php (1 hunks)
  • src/Storage/Device/Local.php (1 hunks)
  • src/Storage/Device/S3.php (1 hunks)
  • tests/Storage/Device/LocalTest.php (1 hunks)
  • tests/Storage/S3Base.php (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
src/Storage/Device/Local.php (2)
src/Storage/Device.php (1)
  • directoryExists (217-217)
src/Storage/Device/S3.php (1)
  • directoryExists (643-657)
tests/Storage/Device/LocalTest.php (3)
tests/Storage/S3Base.php (1)
  • testDirectoryExists (147-165)
src/Storage/Device.php (3)
  • directoryExists (217-217)
  • getPath (85-85)
  • createDirectory (257-257)
src/Storage/Device/Local.php (3)
  • directoryExists (399-402)
  • getPath (63-66)
  • createDirectory (451-460)
src/Storage/Device.php (2)
src/Storage/Device/Local.php (1)
  • directoryExists (399-402)
src/Storage/Device/S3.php (1)
  • directoryExists (643-657)
src/Storage/Device/S3.php (2)
src/Storage/Device.php (1)
  • directoryExists (217-217)
src/Storage/Device/Local.php (1)
  • directoryExists (399-402)
tests/Storage/S3Base.php (9)
tests/Storage/Device/BackblazeTest.php (1)
  • getAdapterType (25-28)
tests/Storage/Device/LinodeTest.php (1)
  • getAdapterType (25-28)
tests/Storage/Device/S3Test.php (1)
  • getAdapterType (28-31)
tests/Storage/Device/DOSpacesTest.php (1)
  • getAdapterType (25-28)
tests/Storage/Device/WasabiTest.php (1)
  • getAdapterType (25-28)
tests/Storage/Device/LocalTest.php (1)
  • testDirectoryExists (87-104)
src/Storage/Device.php (4)
  • directoryExists (217-217)
  • getPath (85-85)
  • write (159-159)
  • delete (192-192)
src/Storage/Device/Local.php (4)
  • directoryExists (399-402)
  • getPath (63-66)
  • write (288-297)
  • delete (338-353)
src/Storage/Device/S3.php (4)
  • directoryExists (643-657)
  • getPath (230-233)
  • write (503-515)
  • delete (527-538)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Build & Unit
🔇 Additional comments (7)
src/Storage/Device.php (1)

211-217: LGTM! Clean API extension.

The abstract method declaration is well-documented and consistent with the existing exists() method pattern. The API contract is clear.

src/Storage/Device/Local.php (1)

393-402: LGTM! Straightforward and correct implementation.

The use of \is_dir() is the appropriate approach for checking directory existence on the local filesystem. The implementation correctly handles both existing and non-existing paths.

tests/Storage/Device/LocalTest.php (1)

87-104: LGTM! Comprehensive test coverage.

The test covers all essential scenarios:

  • Existing directories
  • Non-existing directories
  • Directory creation and subsequent existence check
  • Proper cleanup

The test logic is sound and assertions are appropriate.

src/Storage/Device/S3.php (1)

635-657: LGTM! Correct S3 directory existence check.

The implementation properly handles S3's virtual directory concept by:

  • Normalizing the path to ensure a trailing slash for prefix matching
  • Stripping leading slashes per S3 requirements
  • Efficiently checking for at least one object with the prefix (limit 1)
  • Gracefully handling exceptions by returning false

The approach is consistent with the existing exists() method pattern.

tests/Storage/S3Base.php (3)

23-26: LGTM! Necessary abstract method addition.

The getAdapterType() abstract method is required for the existing testType() method at line 108 and aligns with the pattern of other abstract methods in this base class.


147-165: LGTM! Thorough S3 directory existence testing.

The test comprehensively validates S3's virtual directory semantics:

  • Tests both with and without trailing slashes (important for S3)
  • Verifies existing directories containing files
  • Validates non-existing directories return false
  • Tests nested directory structure detection via prefix matching
  • Includes proper cleanup

The test coverage is excellent and appropriate for S3's object-based storage model.


23-26: Child test classes implement getAdapterType(): change is consistent

All five S3Base subclasses (BackblazeTest, LinodeTest, S3Test, DOSpacesTest, WasabiTest) already define getAdapterType(), so adding it as an abstract method ensures consistency without breaking tests.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ChiragAgg5k ChiragAgg5k changed the title feat: dirExists feat: directoryExists Oct 1, 2025
@ChiragAgg5k ChiragAgg5k closed this Oct 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant